-
Notifications
You must be signed in to change notification settings - Fork 3
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Adding CI #7
Adding CI #7
Conversation
ef2a55e
to
eae0d44
Compare
a0906fe
to
4f3e40b
Compare
Another PR around 1000 lines, but most of it is copied from pymeasure and adjusted accordingly. It's good, that we set it up: I already encountered one incompatibility with python 3.10 (I use 3.11). |
Open:
|
I'd just use ruff, and not flake8 anymore, no sense in duplicating this. Also, consider using mamba instead of conda, it should be much faster (https://github.com/mamba-org/setup-micromamba should be the right action)... it's kinda crazy how much time package/env setup is taking in the pymeasure CI jobs. |
9ad9376
to
c883a1f
Compare
Great work! I have to go through it later..
I would not ignore the init file, but just specifically ignore that clause for the error condition 👍 except (ImportError, LookupError): # pragma: no cover That is more targeted, and the rest of the file remains covered. For teh leco-protocol, you seem to even give testing instructions (I only briefly skimmed). You don't think this should be tested in pytest? Or does just the coverage checking not work there? |
Regarding ruff: should we move that |
I give instructions on how to use these protocol definitions to test an implementation of that protocol. Testing the protocols has two difficulties:
|
Re the |
Coverage calculated needs them. Otherwise it says 0%... |
The reason is, that flake was configured that way I. Pymeasure. I just adapted it. |
coverage of testing code does not make sense, though, does it? It's the testing code that creates coverage after all? |
OK, fair enough! |
Sorry for not being specific enough: Coverage needs init files in the test directory to calculate the coverage of the main source files... EDIT: here the suggestion to add init.py files https://stackoverflow.com/questions/47287721/coverage-py-warning-no-data-was-collected-no-data-collected#52044708 |
I figured it out: You have to install the package editable, then you do not need the init files. |
that's probably just a workaround/hack, but at least the problem's avoided. 👍 |
I fixed all your points (except the ruff configuration):
EDIT: I pushed temporarily (changed in a commit, reversed in the next) a broken message file which was (correctly) shown as broken by mypy. So that works as expected. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good, I had some minor remarks.
Co-authored-by: Christoph Buchner <[email protected]>
Thanks for all the work! Feel free to merge! |
Tries to setup automatic testing and versioning
Closes #4
closes #6